Add new nvtext minhash_permuted API #16756
Conversation
davidwendt
added
2 - In Progress
…o perf-minhash-highmem
I don't think we should remove the names without at least one release of deprecation. |
Agreed
|
So really what we plan to do is change the |
Maybe something like this? diff --git a/python/pylibcudf/pylibcudf/nvtext/minhash.pyx b/python/pylibcudf/pylibcudf/nvtext/minhash.pyx
index f1e012e60e..641af34eab 100644
--- a/python/pylibcudf/pylibcudf/nvtext/minhash.pyx
+++ b/python/pylibcudf/pylibcudf/nvtext/minhash.pyx
@@ -16,6 +16,7 @@ from pylibcudf.libcudf.types cimport size_type
from pylibcudf.scalar cimport Scalar
from cython.operator import dereference
+import warnings
cpdef Column minhash(Column input, ColumnOrScalar seeds, size_type width=4):
@@ -40,6 +41,11 @@ cpdef Column minhash(Column input, ColumnOrScalar seeds, size_type width=4):
Column
List column of minhash values for each string per seed
"""
+ warnings.warn(
+ "Starting in version 25.02, the signature of this function will "
+ "be changed to match pylibcudf.nvtext.minhash_permuted.",
+ DeprecationWarningAnd then maybe add a sentence to the docstring of |
There was a problem hiding this comment.
Very clean code.
Only two doc fixes and a question about shared memory type but nothing blocking. I also noticed several instances where () is used instead of {} for initialization. These are just minor nitpicks, though:
auto const hasher = HashFunction(seed);
auto const hash_str = cudf::string_view(itr, bytes);
|
/merge |
Description
Introduce new nvtext minhash API that takes a single seed for hashing and 2 parameter vectors to calculate the minhash results from the seed hash:
The
seedis used to hash theinputusing rolling set of substringswidthcharacters wide.The hashes are then combined with the values in
parameter_aandparameter_bto calculate a set of 32-bit (or 64-bit) values for each row. Only the minimum value is returned per element ofaandbwhen combined with all the hashes for a row. Each output row is a set of M values whereM = parameter_a.size() = parameter_b.size()This implementation is significantly faster than the current minhash which computes hashes for multiple seeds.
Included in this PR is also the
minhash64_permuted()API that is identical but uses 64-bit values for the seed and the parameter values. Also included are new tests and a benchmark as well as the pylibcudf and cudf interfaces.Checklist